Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement an Event mangement system for Silverstripe CMS #1

Merged
merged 26 commits into from
Nov 17, 2024
Merged

Conversation

maxime-rainville
Copy link
Member

@maxime-rainville maxime-rainville commented Nov 10, 2024

This Pull request provides the initial implementation for the module.

Parent issue

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not adding in static checks using phpstan?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added it back and fix a boat load of warning. Honestly, the only reason why it was on the other module was because Claude put it there without telling me 😅


## Setting up the Event Loop

Because we are using Revolt PHP, you need to run the event loop to process the events.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not completely true if we are using amphp. We can await futures, the EventLoop doesn't need to be explicitly run. This is just very specifically tailored to our use case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess people could manually wait for all the futures. But it seems like a bit of a weird use case.

The main value of this module is that will do things asynchronously.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't be surprised if we come up with a use case where we actually need to just wait for the event, instead of it being completely deferred. Unless your intention for this library is to be always deferred, with no way to one day decide "I need to use this event.. now".

There's also being able chain reactions to events.. like:

Event::queueEvent()->map(function ($result) {
// You can now do something else with the event.. and it's still deferred
});

There'a also

Event::queueEvent()->catch(function ($exception) {
// Maybe I expect this event to fail at times and want to run something else in case
});

and then of course, finally... for use cases that require an event to be acted on immediately:

$result = Event::queueEvent()->await();

for whatever reason. I guess the argument is to just use whatever internal service is being called directly, but, I'd be too lazy to do that and just want to deal with it then and there with the code i have.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
phpstan.neon Outdated Show resolved Hide resolved
src/Event/DataObjectEvent.php Show resolved Hide resolved
tests/php/Service/EventServiceTest.php Outdated Show resolved Hide resolved
Copy link

@marwan38 marwan38 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work Max

@maxime-rainville maxime-rainville merged commit 5430498 into 0 Nov 17, 2024
16 checks passed
@maxime-rainville maxime-rainville deleted the master branch November 17, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants